Skip to content

fix: enhance name splitting to handle "Lastname, Firstname" format - #62122

Open
enjeck wants to merge 3 commits into
masterfrom
fix/comma-seperated-names
Open

fix: enhance name splitting to handle "Lastname, Firstname" format#62122
enjeck wants to merge 3 commits into
masterfrom
fix/comma-seperated-names

Conversation

@enjeck

@enjeck enjeck commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

If an ActiveDirectory provides user names in the "Lastname, Firstname" format, Nextcloud attempts to split this string internally, but it maps the values the wrong way around.
This becomes very obvious in the Contacts app, where the "First name" field is incorrectly populated with the user's last name including the comma (e.g., "Lastname,"), and the "Last name" field contains the user's actual first name

TODO

  • ...

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

@enjeck enjeck self-assigned this Jul 14, 2026
@github-project-automation github-project-automation Bot moved this to 🧭 Planning evaluation (don't pick) in 📝 Productivity team Jul 14, 2026
@enjeck
enjeck force-pushed the fix/comma-seperated-names branch 3 times, most recently from ec835f7 to 5437266 Compare July 14, 2026 18:58
@Antreesy

Antreesy commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

How does it cover cases like this?

  • Jane Doe, MD
  • Martin Luther King, Jr.
  • Dr. Jane Doe

@enjeck
enjeck force-pushed the fix/comma-seperated-names branch from 5437266 to 9a3b882 Compare July 23, 2026 08:28
@enjeck

This comment was marked as resolved.

@Antreesy

Copy link
Copy Markdown
Contributor

More or less that was implemented on Talk client side: https://github.com/nextcloud/spreed/pull/18682/changes

Might be an overkill, but basics to match would be:

  • strip everything in braces
  • strip common suffixes and prefixes, but no to mix with initials
  • if there is a comma, check pattern for how many words are before comma:
    • a b, c - a supposed to be a first name
    • a, b c - b supposed to be a first name

@enjeck
enjeck force-pushed the fix/comma-seperated-names branch from b28fbb6 to 5581ee3 Compare July 24, 2026 01:04
@enjeck enjeck added the 3. to review Waiting for reviews label Jul 24, 2026
Comment thread apps/dav/lib/CardDAV/Converter.php Outdated
Comment thread apps/dav/lib/CardDAV/Converter.php Outdated

@susnux susnux left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it also makes sense to reuse something like https://packagist.org/packages/iliaal/nameparser
And extend the languages not provided. It seems to contain many more entries for suffixes and salutations

enjeck added 2 commits August 12, 2026 06:28
Signed-off-by: Enjeck C. <patrathewhiz@gmail.com>
Signed-off-by: Enjeck C. <patrathewhiz@gmail.com>
@enjeck
enjeck force-pushed the fix/comma-seperated-names branch from 5581ee3 to 89bc56a Compare August 12, 2026 05:28
Signed-off-by: Enjeck C. <patrathewhiz@gmail.com>
@enjeck

enjeck commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Maybe it also makes sense to reuse something like https://packagist.org/packages/iliaal/nameparser

I didn't realize we could add packages without some discussion about it before hand before hand about the supply chain risk. Also, it looks pretty new and shows only 400 installs so far.

@enjeck
enjeck requested a review from susnux August 12, 2026 06:13

@blizzz blizzz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK to follow Talk's approach for consistency. Switching for a dep can be considered later, then it would be good to have a one-way thing to do it across Nextcloud (and its apps).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews

Projects

Status: 🧭 Planning evaluation (don't pick)

Development

Successfully merging this pull request may close these issues.

4 participants